Skip to content

Conversation

Chanwon-Seo
Copy link
Contributor

This PR resolves issue #46969.

Summary

Allows spring.flyway.ignore-migration-patterns to be set to an empty string to align with Flyway's documented behavior for unsetting default migration patterns.

Changes

  • In FlywayAutoConfiguration, removed the condition that prevented an empty list from being passed to Flyway's ignoreMigrationPatterns configuration.
  • This change ensures that setting spring.flyway.ignore-migration-patterns= in application properties works as expected.

Testing

The existing test ignoreMigrationPatternsIsEmpty in FlywayAutoConfigurationTests was used to verify the fix.

Allows 'spring.flyway.ignore-migration-patterns' to be set to an empty
string to align with Flyway's documented behavior for unsetting
default migration patterns.

Signed-off-by: Chanwon-Seo <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 28, 2025
@mhalbritter mhalbritter changed the title Fix Allow empty string for spring.flyway.ignore-migration-patterns Flyway Ignore Migration Patterns setting can't be set to an empty string Aug 28, 2025
Copy link
Member

@wilkinsona wilkinsona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I've left a couple of comments for your consideration.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Aug 29, 2025
@nosan
Copy link
Contributor

nosan commented Aug 29, 2025

I think there are two options to consider for this issue:

The first option is to leave the property in FlywayProperties as it is (@Nullable), simply remove .whenNot(List::isEmpty), and add tests to verify that when the property is not set, the default from Flyway is used.

The second option is to initialize ignoreMigrationPatterns field with a default value, remove .whenNot(List::isEmpty), and write tests to ensure that the default value is aligned with Flyway's default.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 29, 2025
@wilkinsona
Copy link
Member

wilkinsona commented Aug 29, 2025

The second option is to initialize ignoreMigrationPatterns field with a default value, remove .whenNot(List::isEmpty), and write tests to ensure that the default value is aligned with Flyway's default.

This is the better approach. Where possible, it's good for the default value of a Boot property to match the default value of whatever Boot's configuring. That benefits property auto-completion in an IDE, for example. For IDEs that use the metadata (IntelliJ no longer does, AFAIK), we may need to add some manual metadata as I can't remember if the annotation processor understands Collections.singletonList, List.of or the like.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Aug 29, 2025
@nosan
Copy link
Contributor

nosan commented Aug 29, 2025

This is the better approach. Where possible, it's good for the default value of a Boot property to match the default value of whatever Boot's configuring.

In that case the test can verify ...containsExactly(new FluentConfiguration().getIgnoreMigrationPatterns()));

, we may need to add some manual metadata as I can't remember if the annotation processor understands Collections.singletonList, List.of or the like.

It does work for List<String> ignoreMigrationPatterns = Collections.singletonList("*:future");
but not for List<String> ignoreMigrationPatterns = new ArrayList<>(Collections.singletonList("*:future"));

I suggest add the following tests to cover all three scenarios:

@Test
void ignoreMigrationPatternsCorrectlyMapped() {
//"spring.flyway.ignore-migration-patterns=*:missing,*:future"
}

@Test
void ignoreMigrationPatternsWhenEmpty() {
//spring.flyway.ignore-migration-patterns=
}

@Test
void ignoreMigrationPatternsUsesDefaultValuesWhenNotSet() {
//when not set, should be aligned with Flyway's  new FluentConfiguration().getIgnoreMigrationPatterns()
}
	

P.S. Is it somehow possible to tell GitHub not to remove status: waiting-for-feedback?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 29, 2025
@wilkinsona
Copy link
Member

P.S. Is it somehow possible to tell GitHub not to remove status: waiting-for-feedback?

Not at the moment: spring-io/issue-bot#23.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Aug 29, 2025
@Chanwon-Seo
Copy link
Contributor Author

Chanwon-Seo commented Aug 29, 2025

Thank you both, @wilkinsonaand @nosan, for the incredibly helpful and insightful discussion!

I've updated the PR with all the suggested changes. I really appreciate the detailed guidance on aligning the properties with Flyway's defaults and the correct way to test for it.

I've addressed it in ac59c8

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 29, 2025
Removes the `.whenNot(List::isEmpty)` condition to allow clearing the
ignore-migration-patterns property via an empty string.

To avoid affecting existing users, the property's default is now
aligned with Flyway's default of `*:future`

Signed-off-by: Chanwon-Seo <[email protected]>
Removes the `.whenNot(List::isEmpty)` check to allow clearing the property.
The property's default is aligned with Flyway's to avoid breaking
existing behavior, and new tests verify both changes.

Signed-off-by: Chanwon-Seo <[email protected]>
 Reversed actual/expected comparison for ignoreMigrationPatterns, using type-safe ValidatePattern.

Signed-off-by: Chanwon-Seo <[email protected]>
@wilkinsona wilkinsona removed the status: waiting-for-triage An issue we've not yet triaged label Aug 29, 2025
@wilkinsona wilkinsona added type: bug A general bug and removed status: feedback-provided Feedback has been provided labels Aug 29, 2025
@wilkinsona wilkinsona added this to the 3.4.x milestone Aug 29, 2025
@mhalbritter
Copy link
Contributor

Thanks @Chanwon-Seo !

@mhalbritter mhalbritter modified the milestones: 3.4.x, 3.4.10 Sep 1, 2025
dhruv-15-03 pushed a commit to dhruv-15-03/spring-boot that referenced this pull request Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants